[HDX-4406] Number tile conditional color rules (ordered thresholds)#2386
Draft
alex-fedotyev wants to merge 7 commits into
Draft
[HDX-4406] Number tile conditional color rules (ordered thresholds)#2386alex-fedotyev wants to merge 7 commits into
alex-fedotyev wants to merge 7 commits into
Conversation
added 5 commits
May 29, 2026 23:35
Per-row Tooltip.Floating instances got stranded in their Portal when a virtual row unmounted before onMouseLeave fired — a race that occurs when the mouse moves rapidly across a TanStack Virtual list. Fix: replace one Tooltip.Floating per virtual row with a single shared Tooltip.Floating wrapping the whole <tbody>. The floating tooltip now lives on <tbody>, which never unmounts, so its Portal-rendered content can never be left open after the triggering element disappears. Row-level onMouseEnter/onMouseLeave handlers update a shared hoveredRowDescription state; the tooltip's disabled prop gates visibility so rows without a resolved URL (error-toast branch) never show a hint. A tbody-level onMouseLeave acts as a safety net to clear the description if a rapid mouse move causes a row to unmount before its own leave handler fires. Test: adds a regression test that verifies the tooltip disappears on mouseLeave (the stranded-tooltip scenario).
Verifies that the Tooltip.Floating hint appears on row hover and disappears when the mouse moves away, covering the stranded-tooltip regression introduced by the per-row Tooltip.Floating in PR #2321. Changes: - dashboard-table-linking.spec.ts: new 'Tooltip hint appears on hover and disappears on mouse-leave' test. Creates a table tile with a Search row-click action, hovers the first row to confirm the hint appears, then moves the mouse away to confirm it hides cleanly. - DashboardPage.ts: adds getFirstTableRow() and hoverFirstTableRowAndGetTooltip() page-object helpers.
P2 — correctness:
- Store hoveredVirtualIndex (row index) instead of hoveredRowDescription
(string) so the label re-derives via useMemo on every render. If the
virtualiser drops or replaces the hovered row (scroll, auto-refetch,
rapid cursor movement) the new row's action is shown immediately;
stale text from the unmounted row can never persist.
- Rows with url:null or empty description now correctly disable the
tooltip regardless of what the prior hover state was.
P2 — testing:
- Replaced the simple mouseLeave regression test with one that exercises
the actual race: hover index 0 (URL row), then enter index 1 (no-URL
row) without firing mouseLeave on index 0. Asserts tooltip hides by
inspecting the Mantine inline display style on the Portal container.
P3 — maintainability:
- label={hoveredRowDescription} — drop the ?? '' fallback that obscured
the disabled gating relationship (Mantine accepts null as ReactNode).
- disabled={!hoveredRowDescription} — guards against empty-string
descriptions that would mount a zero-width floating tooltip.
- Unconditional onMouseEnter/onMouseLeave on each <tr> with a hoisted
clearHovered useCallback, replacing the conditional handler pattern
that forked JSX unnecessarily.
- Collapse dual comment blocks into one rationale at the Tooltip.Floating
call site; the state declaration now has a single-line pointer.
- Add data-testid="row-action-hint" to the Tooltip.Floating label span
so E2E tests locate the tooltip by stable testid rather than by
hard-coupled copy strings.
Add Grafana-style threshold color rules to number tiles. Users can define an ordered list of conditions in Display Settings; the last matching rule's color wins, falling back to the static tile color, then to the default text color. Schema (common-utils): - Add ColorConditionSchema (discriminated union: gt/gte/lt/lte/between/ eq/neq/contains/startsWith/endsWith/regex) - Add colorRules to SharedChartSettingsSchema (optional, max 10) Resolver (app): - evaluateColorCondition: per-rule evaluation with type guards - resolveConditionalColor: last-match-wins, null-safe UI (app): - ColorRulesEditor: sortable rule list via @dnd-kit/sortable, per-operator value inputs, ColorSwatchInput, add/delete controls - ChartDisplaySettingsDrawer: conditional colors section gated on DisplayType.Number, below existing static color picker - EditTimeChartForm: wire colorRules through displaySettings and handleUpdateDisplaySettings - DBNumberChart: evaluate rules against raw numeric value at render time Tests: - Schema positive/negative tests (common-utils) - evaluateColorCondition + resolveConditionalColor unit tests (app) - ColorRulesEditor RTL tests (add/delete/operator/render) - DBNumberChart integration test: success/warning/error scenario No API schema or external-API changes (separate follow-up ticket).
alex-fedotyev
added
the
ai-generated
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
E2E Test Results✅ All tests passed • 194 passed • 3 skipped • 1272s
Tests ran across 4 shards in parallel. |
…yout Two fixes: 1. Color not applying: ClickHouse returns UInt64 counts as JSON strings when output_format_json_quote_64bit_integers=1. The rawValue passed to resolveConditionalColor was a string like "1251132", causing all numeric operators (gte, gt, lt, etc.) to short-circuit with a false result. Coerce parseable-as-number strings to numbers before evaluation. Test added for the string-coercion path. 2. Layout: rule rows now use fixed widths and center alignment instead of stretching full-width with flex-start offsets. Operator w=80, value w=120 (single) / w=72 each (between), Add rule button left-aligned.
alex-fedotyev
changed the title
Replace references to a competitor's threshold model with neutral terminology: 'last-match-wins' and 'higher-priority rules go last'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds conditional color rules to number tiles. Define an ordered list of conditions; the last matching rule's color wins — so list rules in ascending priority order with the highest-priority rule last. If no rule matches, the tile falls back to its static color (set in the previous AC3 PR), then to the default text color.
The schema is intentionally generic at the shared level so a future table-tile slice can attach per-column rules without a schema change.
What changed
common-utils (schema)
ColorConditionSchema(discriminated union ofgt,gte,lt,lte,between,eq,neq,contains,startsWith,endsWith,regexoperators)colorRules(optional, max 10) toSharedChartSettingsSchemaalongside the existingcolorfieldapp (resolver)
evaluateColorCondition: evaluates a single rule against a runtime value with proper type guards (numeric ops reject strings, string ops reject numbers, bad regex returns false)resolveConditionalColor: iterates rules in order, last match wins, falls back tofallback(the static color) when nothing matchesUInt64counts as JSON strings) are coerced to numbers before rule evaluationapp (UI)
ColorRulesEditorcomponent: sortable rule list via@dnd-kit/sortable, per-operator value inputs (single number, two-number range forbetween, text-or-number foreq/neq),ColorSwatchInputper rule, add/delete buttons; "Add rule" disables at 10ChartDisplaySettingsDrawer: added "Conditional colors" section gated onDisplayType.Number, placed below the existing static color pickerEditTimeChartForm:colorRuleswired throughuseWatch,displaySettingsmemo, andhandleUpdateDisplaySettingsDBNumberChart: resolves color viaresolveConditionalColor(rawValue, config.colorRules, config.color)at render timeTests
evaluateColorConditionper-operator,resolveConditionalColorincluding last-match-wins, string coercion, and the canonical success/warning/error scenarioColorRulesEditorDBNumberChartwithcolor: 'chart-success'+ two rules confirms value 50 -> success, 200 -> warning, 1000 -> error; also covers string UInt64 inputNo changes to
packages/apischemas or external API (separate follow-up ticket, mirrors HDX-4378).Screenshots or video
How to test on Vercel preview
Preview routes: /dashboards
Steps:
References